Fix precision of cpu to millicore and memory to Mi#8409
Fix precision of cpu to millicore and memory to Mi#8409openshift-bot merged 1 commit intoopenshift:masterfrom
Conversation
|
I think this should be p1 fix, but I cannot label. |
|
[test] |
|
Rationale for p1 fix is I will most likely fix the upstream code to fail validation if it gets fractional bytes for memory or sub-millicore precision on CPU |
|
@abhgupta - fyi |
|
LGTM. Note for the UI team this rounds memory to bytes; output will presumably still show full byte precision if it doesn't fall on binary values (1023 bytes won't round to 1Ki). |
|
we discussed this with @danmcp and this is going to be updated to round to On Fri, Apr 8, 2016 at 12:31 PM, Luke Meyer notifications@github.com
|
derekwaynecarr
force-pushed
the
fix_precision
branch
from
4675426 to
bd09e4f
Compare
derekwaynecarr
changed the title
|
@jwforres @danmcp @sosiouxme PTAL Fixes code to round down cpu to nearest millicore with a floor of 1m. |
|
@ncdc - this is a blocker bug fix, can you please review? |
| Amount: multiply(inf.NewDec(int64(float64(memLimit.Value())*cpuBaseScaleFactor), 3), a.config.limitCPUToMemoryRatio), | ||
| Format: resource.DecimalSI, | ||
| // float math is necessary here as there is no way to create an inf.Dec to represent cpuBaseScaleFactor < 0.001 | ||
| // cpu is measured in millicores, so we need to scale and round up the value to nearest millicore scale |
There was a problem hiding this comment.
comment says "round up" but you use RoundDown below?
There was a problem hiding this comment.
will fix comment, round down is correct, comment was missed in my update.
derekwaynecarr
force-pushed
the
fix_precision
branch
from
bd09e4f to
ea48395
Compare
|
Comments updated. |
derekwaynecarr
force-pushed
the
fix_precision
branch
from
ea48395 to
a90ca8f
Compare
|
Evaluated for origin test up to a90ca8f |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/2994/) |
|
Math is fun 😄 LGTM |
|
@smarterclayton - can I have approval and merge? |
|
I approve and [merge]! |
|
blast you AWS! [merge] already |
|
I am relentlessly [merge]ing! |
|
Evaluated for origin merge up to a90ca8f |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/2994/) (Image: devenv-rhel7_3997) |
6 participants
Fixes #8391
Fixes code to round down cpu to nearest millicore with a floor of 1m.
Fixes code to round down memory to nearest Mi with a floor of 1Mi.
@spadgett @jwforres @sspeiche @sosiouxme - PTAL